Skip to content

[stm32] move CMSIS hearder files to common folder#5998

Merged
BernardXiong merged 2 commits into
RT-Thread:masterfrom
mysterywolf:finsh
Jun 4, 2022
Merged

[stm32] move CMSIS hearder files to common folder#5998
BernardXiong merged 2 commits into
RT-Thread:masterfrom
mysterywolf:finsh

Conversation

@mysterywolf

@mysterywolf mysterywolf commented May 25, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

拉取/合并请求描述：(PR description)

[
将CMSIS头文件抽出来 统一放在STM32 libraries文件夹下，防止重复文件一轮叠一轮

已经测试通过无论是直接编译还是dist出来的工程都是没有问题的
]

以下的内容不应该在提交PR时的message修改，修改下述message，PR会被直接关闭。请在提交PR后，浏览器查看PR并对以下检查项逐项check，没问题后逐条在页面上打钩。
The following content must not be changed in the submitted PR message. Otherwise, the PR will be closed immediately. After submitted PR, please use a web browser to visit PR, and check items one by one, and ticked them if no problem.

当前拉取/合并请求的状态 Intent for your PR

必须选择一项 Choose one (Mandatory):

  • 本拉取/合并请求是一个草稿版本 This PR is for a code-review and is intended to get feedback
  • 本拉取/合并请求是一个成熟版本 This PR is mature, and ready to be integrated into the repo

代码质量 Code Quality：

我在这个拉取/合并请求中已经考虑了 As part of this pull request, I've considered the following:

  • 已经仔细查看过代码改动的对比 Already check the difference between PR and old code
  • 代码风格正确，包括缩进空格，命名及其他风格 Style guide is adhered to, including spacing, naming and other styles
  • 没有垃圾代码，代码尽量精简，不包含#if 0代码，不包含已经被注释了的代码 All redundant code is removed and cleaned up
  • 所有变更均有原因及合理的，并且不会影响到其他软件组件代码或BSP All modifications are justified and not affect other components or BSP
  • 对难懂代码均提供对应的注释 I've commented appropriately where code is tricky
  • 本拉取/合并请求代码是高质量的 Code in this PR is of high quality
  • 本拉取/合并使用formatting等源码格式化工具确保格式符合RT-Thread代码规范 This PR complies with RT-Thread code specification

@mysterywolf mysterywolf changed the title Finsh [stm32] move CMSIS hearder files to common folder May 25, 2022
@mysterywolf mysterywolf requested a review from Guozhanxin May 25, 2022 21:28
@Guozhanxin

Guozhanxin commented May 26, 2022

Copy link
Copy Markdown
Member

如果没有特别大的提升，不建议这么修改，之前只是移除一部分cmsis的文件还好，这个pr相当于把cmsis分成了两部分，一部分系统统一管理，一部分各个库管理，还不在一个地方。容易给后期更新、维护造成困扰

1 similar comment
@Guozhanxin

Guozhanxin commented May 26, 2022

Copy link
Copy Markdown
Member

如果没有特别大的提升，不建议这么修改，之前只是移除一部分cmsis的文件还好，这个pr相当于把cmsis分成了两部分，一部分系统统一管理，一部分各个库管理，还不在一个地方。容易给后期更新、维护造成困扰

@mysterywolf

mysterywolf commented May 26, 2022

Copy link
Copy Markdown
Member Author

现在有一些bsp一个BSP就含着一个CMSIS，CMSIS越来越多了。

@mysterywolf

mysterywolf commented May 26, 2022 •
edited
Loading

Copy link
Copy Markdown
Member Author

后期维护不会出现过多困扰的，写一个说明文档就好了。现在为了维护偷懒 导致仓库里存了大量了无用、重复的文件
而且STM32的文件结构会误导其他厂商BSP的提交

@mysterywolf mysterywolf requested a review from liukangcc May 30, 2022 22:44
@liukangcc liukangcc added the +1 Agree +1 label May 31, 2022
@supperthomas

supperthomas commented Jun 3, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

这个改动非常好，我之前也觉得很多头文件很大（几M多），也重复。
就是提醒一下，我之前好像遇到过因为这个CMSIS.h文件版本不一样（文件名一样，内容不一样），导致代码运行逻辑不一样的。不知道这里的版本到时候好不好控制，也不知道这样改了之后，有些STM32代码运行逻辑会不会有所不同。
改动方向支持的，建议要加强版本维护。或者是否考虑软件包来管理这些文件
+1

@supperthomas supperthomas added BSP: STM32 BSP related with ST/STM32 wait_+2 wait for "+2" to confirm labels Jun 3, 2022
@mysterywolf mysterywolf added +2 Agree +2 and removed wait_+2 wait for "+2" to confirm labels Jun 3, 2022
@BernardXiong BernardXiong merged commit b197b50 into RT-Thread:master Jun 4, 2022
@mysterywolf mysterywolf deleted the finsh branch June 4, 2022 20:35
@mysterywolf mysterywolf mentioned this pull request Jun 16, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Guozhanxin Guozhanxin Awaiting requested review from Guozhanxin
@liukangcc liukangcc Awaiting requested review from liukangcc

Assignees

No one assigned

Labels

BSP: STM32 BSP related with ST/STM32 important +1 Agree +1 +2 Agree +2

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@mysterywolf @Guozhanxin @supperthomas @BernardXiong @liukangcc